Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Examples: Fix build on master #9759

Merged
merged 5 commits into from
Feb 6, 2020
Merged

Examples: Fix build on master #9759

merged 5 commits into from
Feb 6, 2020

Conversation

ndelangen
Copy link
Member

Issue: https://app.netlify.com/sites/storybookjs/deploys/5e3a8b3054f52300084af24c

The examples aren't building on master, because of some version mismatch with MiniCssExtractPlugin & cra.

What I did

  • I REMOVED the MiniCssExtractPlugin for CRA. It's only giving grief :(

@ndelangen ndelangen added bug cra Prioritize create-react-app compatibility labels Feb 5, 2020
@ndelangen ndelangen self-assigned this Feb 5, 2020
@ndelangen
Copy link
Member Author

@shilman the changes in the lockfile shouldn't affect users.

The snapshots & typings issues are just caused by a minor version bumps of some deps.

The removal of the MiniCSSExtract plugin I did, because I'm tired of that thing breaking.

@ndelangen
Copy link
Member Author

This will make it so the examples on netlify start working

shilman
shilman approved these changes Feb 6, 2020
View reviewed changes
Copy link
Member

@shilman shilman left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

N/A

shilman
shilman requested changes Feb 6, 2020
View reviewed changes
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is removing the CSS minification actually necessary to get this working? If not, can you put it in separate PR? That's a user-facing change on master

@ndelangen
Copy link
Member Author

@shilman I was not able to get it working without disabling the CSSMinificationPlugin :(

shilman
shilman approved these changes Feb 6, 2020
View reviewed changes
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@shilman shilman changed the title FIX master Examples: Fix build on master Feb 6, 2020
@ndelangen ndelangen merged commit 7d5a408 into master Feb 6, 2020
@ndelangen ndelangen deleted the fix/master branch February 6, 2020 13:04
@mtliendo
Copy link

mtliendo commented Feb 18, 2020 •
edited
Loading

--Edit--

Disregard, my issue was due to not importing a global scss file in preview.js 🤦‍♂

Since upgrading to 5.3.13, my scss files are no longer loading and thus my build is breaking. I believe it has something to do with this being removed.

image

Downgrading to 5.3.12 resolves my issue instantly. Additionally, upgrading to 6-alpha breaks the build with the same error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shilman shilman shilman approved these changes

@mrmckeb mrmckeb Awaiting requested review from mrmckeb

@kazupon kazupon Awaiting requested review from kazupon kazupon is a code owner

@thomasbertet thomasbertet Awaiting requested review from thomasbertet thomasbertet is a code owner

@tmeasday tmeasday Awaiting requested review from tmeasday tmeasday is a code owner

@alexandrebodin alexandrebodin Awaiting requested review from alexandrebodin alexandrebodin is a code owner

@alterx alterx Awaiting requested review from alterx alterx is a code owner

@igor-dv igor-dv Awaiting requested review from igor-dv igor-dv is a code owner

@leonrodenburg leonrodenburg Awaiting requested review from leonrodenburg leonrodenburg is a code owner

@theinterned theinterned Awaiting requested review from theinterned theinterned is a code owner

Assignees

@ndelangen ndelangen

Labels
bug cra Prioritize create-react-app compatibility
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ndelangen @mtliendo @shilman